fix(moderation): add role hierarchy check to all mod actions#1228
fix(moderation): add role hierarchy check to all mod actions#1228
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a Discord role hierarchy safety check to the generic moderation entrypoint so that all moderation actions respect role ordering before executing. Sequence diagram for moderation action with role hierarchy checksequenceDiagram
actor Moderator
participant DiscordGuild as DiscordGuild
participant Bot as ModerationModule
participant ModerationService
Moderator->>Bot: moderate_user(ctx, user, case_type, **kwargs)
Bot->>DiscordGuild: get ctx.guild, ctx.author, user.top_role
alt User_is_guild_member_and_role_not_lower
Bot-->>Moderator: _respond(ctx, cannot_moderate_higher_role)
Note over Bot,Moderator: Early return, no moderation action executed
else User_role_lower_than_moderator
Bot->>ModerationService: execute_moderation_action(ctx, case_type, user, **kwargs)
ModerationService-->>Moderator: Moderation result/confirmation
end
Updated class diagram for moderation module role hierarchy checkclassDiagram
class ModerationModule {
+moderate_user(ctx, user, case_type, kwargs)
+_respond(ctx, message)
-moderation : ModerationService
}
class ModerationService {
+execute_moderation_action(ctx, case_type, user, kwargs)
}
class DiscordMember {
+top_role : Role
}
class Context {
+guild : Guild
+author : DiscordMember
}
class Guild {
+owner : DiscordMember
}
class Role {
}
ModerationModule --> ModerationService : uses
ModerationModule --> Context : receives
ModerationModule --> DiscordMember : moderates
Context --> Guild : belongs_to
DiscordMember --> Role : has
Guild --> DiscordMember : owner
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 17 minutes and 55 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdded a role hierarchy validation in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR adds role hierarchy checking to moderation actions, which addresses the reported issue. However, there's a critical logic gap that needs to be addressed before merge.
Critical Issue
The role hierarchy validation only checks if the moderator can perform the action, but doesn't verify if the bot itself has sufficient role permissions. This will cause moderation actions to fail silently after passing validation when the target user's role is equal to or higher than the bot's role.
Recommendation
Add an additional check to validate the bot's role hierarchy relative to the target user. This prevents confusing error states where the command appears to succeed but the Discord API rejects the action due to insufficient bot permissions.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider extracting the role hierarchy check into a shared helper (e.g.,
can_moderate(ctx.author, user, ctx.guild)) so that any other moderation entry points can reuse the same logic and stay consistent if the rules need to evolve. - You may also want to mirror this hierarchy check against the bot’s own top role (ensuring the bot can actually act on the target) and reuse that in the same helper to keep user- and bot-side constraints in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the role hierarchy check into a shared helper (e.g., `can_moderate(ctx.author, user, ctx.guild)`) so that any other moderation entry points can reuse the same logic and stay consistent if the rules need to evolve.
- You may also want to mirror this hierarchy check against the bot’s own top role (ensuring the bot can actually act on the target) and reuse that in the same helper to keep user- and bot-side constraints in one place.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
📚 Documentation Preview
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (42.25%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #1228 +/- ##
=======================================
Coverage 42.25% 42.25%
=======================================
Files 221 221
Lines 18402 18409 +7
Branches 2428 2431 +3
=======================================
+ Hits 7775 7778 +3
- Misses 10627 10631 +4 ☔ View full report in Codecov by Sentry. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Confidence score: 3/5
- There is a concrete regression risk in
src/tux/modules/moderation/__init__.py: the hierarchy check covers the invoking moderator vs. target, but not the bot vs. target role position. - Because this is severity 6/10 with high confidence (9/10), moderation actions can still be attempted against members the bot cannot act on, which may cause failed commands or runtime permission errors for users.
- This PR looks close, but this issue is user-facing enough to warrant a cautious merge score until the bot-role hierarchy validation is added.
- Pay close attention to
src/tux/modules/moderation/__init__.py- missing bot-vs-target role checks can break moderation actions on higher-role members.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/tux/modules/moderation/__init__.py">
<violation number="1" location="src/tux/modules/moderation/__init__.py:112">
P2: This hierarchy check validates the invoking moderator's role against the target but doesn't verify the **bot's** role position relative to the target. If the target member's top role is equal to or higher than the bot's top role, `execute_moderation_action` will proceed past this check but then fail with a `discord.Forbidden` at the API level, producing a confusing error. Add a second check for `ctx.guild.me.top_role <= user.top_role` and return a clear message like "I cannot moderate this member because their role is equal to or higher than mine."</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/tux/modules/moderation/__init__.py (1)
100-112: Consider protecting the guild owner as a moderation target.The check correctly allows the guild owner to moderate anyone (
ctx.author != ctx.guild.ownerexempts them). However, it doesn't prevent moderators from attempting to moderate the guild owner. If the guild owner has a lower-positioned role than the moderator (unusual but possible),user.top_role >= ctx.author.top_rolecould beFalse, allowing the action to proceed—only to fail at the Discord API level.Adding an explicit check for the target being the guild owner provides clearer UX and defensive coding:
♻️ Proposed enhancement
# Role hierarchy check: only applies when the target is a guild member if ( ctx.guild and isinstance(user, discord.Member) and isinstance(ctx.author, discord.Member) + ): + # Guild owner cannot be moderated by anyone + if user == ctx.guild.owner: + await self._respond( + ctx, + "You cannot moderate the server owner.", + ) + return + + # Non-owners cannot moderate members with equal or higher roles + if ( - and ctx.author != ctx.guild.owner - and user.top_role >= ctx.author.top_role - ): - await self._respond( - ctx, - "You cannot moderate a member with an equal or higher role than yours.", - ) - return + ctx.author != ctx.guild.owner + and user.top_role >= ctx.author.top_role + ): + await self._respond( + ctx, + "You cannot moderate a member with an equal or higher role than yours.", + ) + returnBased on learnings: "Implement role-based permissions checks in Discord command modules"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tux/modules/moderation/__init__.py` around lines 100 - 112, Add an explicit guard that prevents moderating the guild owner: before the existing role-hierarchy block in the moderation command (where ctx, user and ctx.guild are available and the current check is performed), check if user is the guild owner (compare user == ctx.guild.owner or user.id == ctx.guild.owner_id) and call self._respond with a clear message and return; keep this check above or alongside the existing role comparison so attempts to moderate the guild owner are rejected early and consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/tux/modules/moderation/__init__.py`:
- Around line 100-112: Add an explicit guard that prevents moderating the guild
owner: before the existing role-hierarchy block in the moderation command (where
ctx, user and ctx.guild are available and the current check is performed), check
if user is the guild owner (compare user == ctx.guild.owner or user.id ==
ctx.guild.owner_id) and call self._respond with a clear message and return; keep
this check above or alongside the existing role comparison so attempts to
moderate the guild owner are rejected early and consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 034811bd-7f1e-491e-bc76-05051828881f
📒 Files selected for processing (1)
src/tux/modules/moderation/__init__.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Run All Tests (3.13.11)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Seer Code Review
- GitHub Check: Dependencies
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/rules.mdc)
**/*.py: Follow Python code style guide and best practices as defined in core/style-guide.mdc
Follow error handling patterns as defined in error-handling/patterns.mdc
Follow loguru logging patterns as defined in error-handling/logging.mdc
Follow Sentry integration patterns as defined in error-handling/sentry.mdc
**/*.py: Use strict type hints withType | Nonesyntax instead ofOptional[Type]in Python files
Use NumPy docstrings in Python files
Prefer absolute imports; relative imports allowed only within the same module in Python files
Organize imports in Python files in the following order: stdlib → third-party → local, with blank lines between groups
Maintain 88 character line length in Python files
Use snake_case for functions and variables, PascalCase for classes, and UPPER_CASE for constants in Python
Always add imports to the top of Python files unless absolutely necessary
Keep individual Python files to a maximum of 1600 lines
Prefer one class or function per Python file when possible for improved maintainability
Use descriptive filenames with clear, purpose-driven names in Python files
Use custom exceptions for business logic errors with context and meaningful user messages
Never store secrets or sensitive credentials in code; use .env files and environment variables
Validate all user inputs at code boundaries in Python files
Type hints must be complete in pull requests
Docstrings must be provided for all public APIs in Python files
Files:
src/tux/modules/moderation/__init__.py
**/modules/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/modules/**/*.py: Use hybrid commands (slash + traditional) in Discord command modules
Implement role-based permissions checks in Discord command modules
Implement cooldowns and rate limiting for Discord commands
Use lazy loading to load modules and plugins on demand
Files:
src/tux/modules/moderation/__init__.py
🧠 Learnings (1)
📚 Learning: 2026-03-08T15:17:13.232Z
Learnt from: CR
Repo: allthingslinux/tux PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-08T15:17:13.232Z
Learning: Applies to **/modules/**/*.py : Implement role-based permissions checks in Discord command modules
Applied to files:
src/tux/modules/moderation/__init__.py
🔇 Additional comments (1)
src/tux/modules/moderation/__init__.py (1)
100-112: Role hierarchy check implementation looks correct.The core logic properly addresses the issue from
#1227:
- Allows guild owners to bypass the role check (can moderate anyone)
- Blocks moderation when target's
top_role >= author's top_role- Only applies when both parties are guild members
- Correctly placed before
execute_moderation_actionwhich has no internal role guards (confirmed by the coordinator snippet)
- Use owner_id instead of owner (avoids None if member uncached) - Add bot-vs-target role check to prevent discord.Forbidden errors
1 participant
Closes #1227
Pull Request
Description
Provide a clear summary of your changes and reference any related issues. Include the motivation behind these changes and list any new dependencies if applicable.
If your PR is related to an issue, please include the issue number below:
Related Issue: Closes #
Type of Change:
Guidelines
My code follows the style guidelines of this project (formatted with Ruff)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if needed
My changes generate no new warnings
I have tested this change
Any dependent changes have been merged and published in downstream modules
I have added all appropriate labels to this PR
I have followed all of these guidelines.
How Has This Been Tested? (if applicable)
Please describe how you tested your code. e.g describe what commands you ran, what arguments, and any config stuff (if applicable)
Screenshots (if applicable)
Please add screenshots to help explain your changes.
Additional Information
Please add any other information that is important to this PR.
Summary by Sourcery
Bug Fixes: